Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(core): clarify output of test_unicode #11572

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

srl295
Copy link
Member

@srl295 srl295 commented May 27, 2024

  • group output by functional area

#10183

Sample output:

----------------------------------- stdout -----------------------------------
= test_all
== load_json loading /Users/srl295/src/keymanapp/keyman/core/build/mac-x86_64/release/tests/unit/ldml/nodeversions.json
== load_json loading /Users/srl295/src/keymanapp/keyman/core/build/mac-x86_64/release/tests/unit/ldml/package.json
= get_block_unicode_ver load /Users/srl295/src/keymanapp/keyman/core/build/mac-x86_64/release/tests/unit/ldml/Blocks.txt
6
== test: test_unicode_versions
ICU Versions:
* 73.1	..linked from C++
* 74.2	..in Node.js

Unicode Versions:
* 15.0	..in ICU linked from C++
* 15.1	..in ICU in Node.js
* 15.1.0	..in Keyman repo Blocks.txt

Node.js
* "18.20.2"	Actual version of Node.js
* ^18.x	Version of Node.js requested by package.json

All OK!

@keymanapp-test-bot skip

- group output by functional area

#10183
@srl295 srl295 self-assigned this May 27, 2024
@srl295 srl295 requested review from ermshiperete and removed request for mcdurdin and rc-swag May 27, 2024 21:43
@srl295 srl295 added this to the A18S3 milestone May 27, 2024
@github-actions github-actions bot added core/ Keyman Core test Any acceptance test issue labels May 27, 2024
@srl295 srl295 requested a review from mcdurdin May 27, 2024 21:44
Comment on lines 90 to 96
std::string result = block_line.substr(prefix.length());
const std::string txt_suffix = ".txt"; // trim off this suffix
auto txt_pos = result.find(txt_suffix, 0);
if (txt_pos != std::string::npos) {
result.resize(txt_pos);
}
return result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe needs a comment! It drops the ".txt" suffix that is present in the original file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous output without this change:

block_unicode_ver 15.1.0.txt

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, a comment would be nice to clarify why this is being done.

Comment on lines 90 to 96
std::string result = block_line.substr(prefix.length());
const std::string txt_suffix = ".txt"; // trim off this suffix
auto txt_pos = result.find(txt_suffix, 0);
if (txt_pos != std::string::npos) {
result.resize(txt_pos);
}
return result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, a comment would be nice to clarify why this is being done.

@srl295 srl295 merged commit b7de602 into master Jun 4, 2024
18 checks passed
@srl295 srl295 deleted the test/core/10183-clarify-unicode-test branch June 4, 2024 13:44
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 18.0.48-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core/ Keyman Core test Any acceptance test issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants